Allow promoted readonly properties to be reassigned once in constructor#20996
Open
nicolas-grekas wants to merge 9 commits intophp:masterfrom
Open
Allow promoted readonly properties to be reassigned once in constructor#20996nicolas-grekas wants to merge 9 commits intophp:masterfrom
nicolas-grekas wants to merge 9 commits intophp:masterfrom
Conversation
1667954 to
e8009a7
Compare
Contributor
Author
|
Implementation is now ready. |
Contributor
Author
|
RFC published at https://wiki.php.net/rfc/promoted_readonly_constructor_reassign |
e8009a7 to
3ccacde
Compare
f36150c to
e051b93
Compare
TimWolla
reviewed
Feb 18, 2026
Member
TimWolla
left a comment
There was a problem hiding this comment.
A number of comments for the tests. Didn't yet look at all of them.
Replace the IS_PROP_CPP_REINITABLE property flag and IS_OBJ_CTOR_CALLED object flag with the existing IS_PROP_REINITABLE flag, following the same try/finally pattern already used by __clone(). How it works: - On CPP implicit initialization, if the current execute frame's scope matches prop_info->ce (no stack walk needed), set IS_PROP_REINITABLE instead of clearing all property flags. This opens a one-shot reassignment window. - zend_leave_helper clears IS_PROP_REINITABLE from all PROMOTED|READONLY properties of the constructor's scope on exit, gated on ZEND_CALL_HAS_THIS|ZEND_ACC_CTOR (covers both `new Foo()` and `parent::__construct()` calls, not just ZEND_CALL_RELEASE_THIS). - zend_is_readonly_property_modifiable() simplified to a single IS_PROP_REINITABLE check; zend_is_in_declaring_constructor() and the call-stack walk helper removed entirely. A repeated __construct() call cannot bypass readonly because the property is already past IS_PROP_UNINIT, so IS_PROP_REINITABLE is never re-set; no object-level tracking flag is needed. This saves two flag bits (property and object extra_flags) and removes ~30 lines of stack-walk code.
d5b3e4c to
67be986
Compare
TimWolla
reviewed
Feb 19, 2026
TimWolla
reviewed
Feb 19, 2026
7a3ef4e to
003b60a
Compare
84228eb to
425081c
Compare
TimWolla
reviewed
Feb 19, 2026
b5c75b3 to
881a8e9
Compare
881a8e9 to
74406a1
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
I find it quite annoying that readonly properties play badly with CPP (constructor property promotion).
Doing simple processing of any argument before assigning it to a readonly property forces opting out of CPP.
This PR allows setting once a readonly property in the body of a constructor after the property was previously set using CPP.
Before this PR (CPP not possible):
After this PR:
This allows keeping properties declaration in its compact form.
I'll submit an RFC of course but would also welcome early feedback here before.